Skip to content

Conversation

Ioannis
Copy link
Contributor

@Ioannis Ioannis commented Apr 11, 2022

Top links on the index view

@Ioannis Ioannis requested a review from arlen April 11, 2022 16:10
@Ioannis Ioannis marked this pull request as draft April 11, 2022 16:10
@Ioannis Ioannis marked this pull request as ready for review April 28, 2022 06:18
@Ioannis Ioannis force-pushed the CFM-143_DRY_Refactor_topLinks_and_indexActions_make_use_of_an_element_or_MenuHelper branch from cf4dc34 to cd6ac46 Compare April 28, 2022 07:17
@arlen
Copy link
Contributor

arlen commented Apr 29, 2022

Hi, Ioannis - I agree with the heart of your question in the code: "Why only with add i have toplinks???" - I believe what was happening here is that "add" was all we started with (much more to come, of course), and then "add" permissions were adequate to determine if the others were to be shown. This is not correct going forward. We should either change the test for "add" to only contain the add link or we should just include add with the other toplinks so that all is consistent.

@Ioannis Ioannis force-pushed the CFM-143_DRY_Refactor_topLinks_and_indexActions_make_use_of_an_element_or_MenuHelper branch from cd6ac46 to 9b995a6 Compare May 1, 2022 09:43
@Ioannis
Copy link
Contributor Author

Ioannis commented May 1, 2022

@arlen I agree with the approach and i removed the comment. If you are okay with the imported functionality for now, let's merge the PR, since we will be making more changes in the near future.

@Ioannis Ioannis force-pushed the CFM-143_DRY_Refactor_topLinks_and_indexActions_make_use_of_an_element_or_MenuHelper branch from 67ac095 to 08fa6ac Compare May 1, 2022 10:52
@Ioannis Ioannis force-pushed the CFM-143_DRY_Refactor_topLinks_and_indexActions_make_use_of_an_element_or_MenuHelper branch from 0d54a79 to ca17e3c Compare May 4, 2022 13:54
@arlen arlen merged commit 68b0e7c into COmanage:develop May 4, 2022
@Ioannis Ioannis deleted the CFM-143_DRY_Refactor_topLinks_and_indexActions_make_use_of_an_element_or_MenuHelper branch May 4, 2022 17:16
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants